docs: use Corepack instead of npm i -g yarn in theme setup#122
docs: use Corepack instead of npm i -g yarn in theme setup#122Boy132 merged 2 commits intopelican-dev:mainfrom
npm i -g yarn in theme setup#122Conversation
Updated instructions for theme setup to include enabling Corepack.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughREADME updated: the "themes" setup now recommends enabling Node.js Corepack ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
21-21: Consider adding context about whensudois needed.The
corepack enablecommand is correct. However, the official Node.js documentation does not explicitly mentionsudoas required—its necessity depends on how Node.js was installed and file permissions on the binary directory. For clarity, consider adding a note explaining thatsudomay be needed for system-wide Node.js installations (such as via NodeSource) but might not be necessary for user-level installations.📝 Optional: Add context about sudo requirement
-sudo corepack enable +sudo corepack enable # May require sudo if Node.js is installed system-wide; not needed for user-level installations🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 21, Add a short clarification next to the "corepack enable" instruction explaining that using sudo may be necessary only for system-wide Node.js installations (e.g., installed via NodeSource or when the global binary directory is owned by root) but is not required for user-level installs; update the README line with "corepack enable" to include a parenthetical or one-sentence note indicating this and suggesting to try without sudo first and use sudo only if permission errors occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Line 21: Add a short clarification next to the "corepack enable" instruction
explaining that using sudo may be necessary only for system-wide Node.js
installations (e.g., installed via NodeSource or when the global binary
directory is owned by root) but is not required for user-level installs; update
the README line with "corepack enable" to include a parenthetical or
one-sentence note indicating this and suggesting to try without sudo first and
use sudo only if permission errors occur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abac13d5-3bc1-408b-af52-4d6fe67a1f9b
📒 Files selected for processing (1)
README.md
📜 Review details
🔇 Additional comments (1)
README.md (1)
15-15: The documentation statement that Corepack ships with Node.js 22+ is accurate and confirmed by nodejs.org documentation across multiple Node.js 22 versions (22.0.0, 22.1.0, 22.2.0, 22.6.0, 22.11.0). No changes needed.
Co-authored-by: Boy132 <Boy132@users.noreply.github.com>
2 participants
The theme setup snippet in
README.mdcurrently installs Yarn vianpm i -g yarn, which requires a global npm write and is the legacy way to get Yarn on Node.Node.js 22 ships with Corepack, the officially supported mechanism for managing package managers (Yarn, pnpm, ...). Enabling Corepack is a single command, avoids the global npm install, and makes the Yarn version pinned per-project (via the
packageManagerfield in a theme'spackage.json) rather than whatever happens to be globally installed on the host.Changes
## Additional setup for themesparagraph mentions Corepack instead of a global Yarn install.npm i -g yarnwithsudo corepack enable.No other changes; the NodeSource install step is untouched.
Summary by CodeRabbit